Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Get unmined ancestors #648

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: Get unmined ancestors #648

wants to merge 3 commits into from

Conversation

boecklim
Copy link
Collaborator

@boecklim boecklim commented Nov 14, 2024 •
edited
Loading

Description of Changes

The recursive search for unmined ancestors lead to sometimes long response times when submitting requests with the cumulative fee validation. The bitcoin node function GetMempoolAncestors offers an easy to use and fast alternative to return all unmined ancestors to a transaction.

  • Instead of recursively searching for unmined ancestors, use the node function GetMempoolAncestors from bitcoin node
  • Move finder out of internal package

Testing Procedure

Describe the tests you've added or any testing steps you've taken.

  • I have added new unit tests
  • All tests pass locally
  • I have tested manually in my local environment

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have updated CHANGELOG.md with my changes

@boecklim boecklim marked this pull request as ready for review November 14, 2024 14:10
@boecklim boecklim self-assigned this Nov 14, 2024
kuba-4chain
kuba-4chain reviewed Nov 15, 2024
View reviewed changes
cmd/arc/services/api.go Outdated Show resolved Hide resolved
internal/validator/default/helpers.go Outdated Show resolved Hide resolved
internal/validator/default/helpers.go Outdated Show resolved Hide resolved
@arkadiuszos4chain
Copy link
Collaborator

arkadiuszos4chain commented Nov 15, 2024 •
edited
Loading

If tx_finder is no longer an internal code of the API:

  1. move it to ./internal/tx_finder
  2. move the validator.TxFinder interface (and finders code) to ./internal/tx_finder
  3. NodeClient interace from tx_finder can either be moved to ./internal/node_client/ or removed entirely
  4. Finder could be injected into CachedFinder

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Nov 15, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
78.2% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arkadiuszos4chain arkadiuszos4chain arkadiuszos4chain approved these changes

@kuba-4chain kuba-4chain kuba-4chain approved these changes

Assignees

@boecklim boecklim

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@boecklim @arkadiuszos4chain @kuba-4chain